@seriousme Out of curiosity did you also did a quick benchmark?

@seriousme Out of curiosity did you also did a quick benchmark?

No I didn't. The challenge for me is to decide what the test should look like.
E.g. a test with a single retained topic (or a few) might be great as a microbenchmark but not really predictive for real life trafic.
Similarly:

• how many subscriptions does a single client on average use and how many of those contain wildcards?
• is the average retained topic tree narrow and deep, or shallow and wide
• how many topics are on average in the retained topic tree (with few topics , Keys() might be faster)
• do we expect a few clients or a few thousand clients in parallel (resulting in resource contention in Redis)
• should we test on single nodejs proces, multiple nodejs processes on the same host, multiple on multiple hosts
• single node redis instance, redis cluster on a single node, redis cluster on multiple nodes.

So all in all there are so many variables involved that I doubt we can make a generic statement on "performance" 😉
The most likely answer will be 'it depends' however for certain uses cases (e.g. topics without wildcards) there can be serious benefits as dicussed in #122 .

If you are unsure whether we will break performance, we could make it an option if you want.
Or stay with the existing Keys() solution as apparently no one complains about the performance of that.

It was fun to solve this puzzle.

Kind regards,
Hans
ps. I know that it's probably not possible but some statistics on how people actually use Aedes would be nice to guide these kinds of decisions.

@seriousme no worries I agree, I think anyway this is much better compared to the solution used before as keys() shouldn't be used on production as stated in redis docs so let's merge this and thanks 🙏🏼

@robertsLando
While explaining the code to a friend I noticed that a small bug got in:

This should give the smallest value for wildcard position, but now it gives the biggest.

e.g. hello/+/world/# now gives 14 (the # ) where it should have returned 6 (the + )

I'll fix it tonight and try to add a test to prevent regression in the future.

Kind regards,
Hans

Oh I totally missed that! My fault, I think adding some tests with different patterns would be safer yeah

The fix is in #123, matching retained messages lives in abstract.js, so I will try to add a pattern there.
Unfortunately that means having to release all 3 again , sorry :-(

No worries :)